Skip to content

(PA-3526) Ruby 2.7.2 performance patches #414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Apr 27, 2021
Merged

(PA-3526) Ruby 2.7.2 performance patches #414

merged 3 commits into from
Apr 27, 2021

Conversation

@jpogran
Copy link
Contributor

@jpogran jpogran commented Jan 19, 2021 •
edited by joshcooper
Loading

  • (PA-3526) Patch Ruby 2.7.2 Avoid realpath in require

@jpogran jpogran force-pushed the ruby-2.7.2-perf-patches branch 2 times, most recently from d44cfdd to d0c3180 Compare January 22, 2021 16:35
@jpogran jpogran changed the title ruby 2.7.2 perf patches Ruby 2.7.2 performance patches Jan 22, 2021
@jpogran jpogran added do not merge enhancement New feature or request labels Jan 22, 2021
lucywyman
lucywyman approved these changes Jan 28, 2021
View reviewed changes
Copy link
Contributor

@lucywyman lucywyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully built an x64 MSI, installed on Windows 2019 just fine, and was able to create a project, run ruby-language tasks (facts), and run Puppet language plans.

@lucywyman lucywyman mentioned this pull request Feb 1, 2021
Closed
2 tasks
@jpogran jpogran force-pushed the ruby-2.7.2-perf-patches branch from d0c3180 to 4c70d16 Compare February 11, 2021 17:46
@jpogran jpogran force-pushed the ruby-2.7.2-perf-patches branch from 4c70d16 to d16928c Compare March 10, 2021 16:47
@mihaibuzgau
Copy link
Contributor

Puppet runtime builds for Puppet Agent 6.x for all the supported platforms are available here: http://builds.delivery.puppetlabs.net/puppet-runtime/d16928cda5c70f0548a97d7acdda6a10791dd5c4/

@mihaibuzgau
Copy link
Contributor

Puppet Agent runtime for main build passed on all the supported. Artifacts are available here: http://builds.delivery.puppetlabs.net/puppet-runtime/d16928cda5c70f0548a97d7acdda6a10791dd5c4/

@mihaibuzgau
Copy link
Contributor

tests passed on all platforms for Puppet 6.x: https://jenkins-master-prod-1.delivery.puppetlabs.net/view/Adhoc/job/platform_puppet-agent-extra_puppet-agent-integration-suite_adhoc-ad_hoc/940/

@jpogran
Copy link
Contributor Author

jpogran commented Mar 16, 2021

This is great news! I am working on getting a PDK build tested with these patches

@mihaibuzgau
Copy link
Contributor

Tests passed on Puppet Agent main (7.x) with this change: https://jenkins-master-prod-1.delivery.puppetlabs.net/view/Adhoc/job/platform_puppet-agent-extra_puppet-agent-integration-suite_adhoc-ad_hoc/942/

@lucywyman
Copy link
Contributor

Hmm, I'm trying to build PDK on Windows 2019 on this PR, and am running into a strange error:

configure: error: dtrace(1) is missing
Makefile:345: recipe for target 'ruby-2.4.10-configure' failed
make: *** [ruby-2.4.10-configure] Error 1

Anything I'm obviously doing wrong?

@lucywyman
Copy link
Contributor

🎉 🌮 🎉 Got a successful run for PDK!!

@lucywyman lucywyman marked this pull request as ready for review March 31, 2021 00:02
@lucywyman lucywyman requested a review from a team March 31, 2021 00:02
@lucywyman lucywyman requested a review from a team as a code owner March 31, 2021 00:02
@joshcooper
Copy link
Collaborator

Yay! Could you reference PA-3526 in the commits. We're definitely going to want to mention this in release notes

@jpogran
Copy link
Contributor Author

jpogran commented Mar 31, 2021

@joshcooper will do!

@lucywyman lucywyman force-pushed the ruby-2.7.2-perf-patches branch from 0ae0e4a to 87d67bf Compare March 31, 2021 14:13
@jpogran jpogran force-pushed the ruby-2.7.2-perf-patches branch from 9c89df2 to d815fb3 Compare March 31, 2021 16:11
@jpogran
Copy link
Contributor Author

jpogran commented Mar 31, 2021

Commit messages amended

joshcooper
joshcooper approved these changes Mar 31, 2021
View reviewed changes
Copy link
Collaborator

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpogran awesome thanks! Could you file a PA ticket so we remember to double check/revert commit 6a6886d when moving to ruby 2.7.3+ as it looks like that issue is fixed in upstream ruby (but not yet released): ruby/ruby@99b4f20

Did you run into issues with the ruby patch for https://bugs.ruby-lang.org/issues/15856? I found it to be somewhat helpful in puppet, but don't remember exact numbers.

@jpogran
Copy link
Contributor Author

jpogran commented Mar 31, 2021

Ticket https://tickets.puppetlabs.com/browse/PA-3691 created to track removing the patch when upstream ruby is merged into the released 2.7

@jpogran
Copy link
Contributor Author

jpogran commented Mar 31, 2021

Did you run into issues with the ruby patch for https://bugs.ruby-lang.org/issues/15856? I found it to be somewhat helpful in puppet, but don't remember exact numbers.

I did find it to be helpful, but had trouble combining it with these patches in vanagon. I decided to first try to merge what was already in use in upstream ruby before attempting to reconcile the differences.

@lucywyman
Copy link
Contributor

\o/ Any other todo items left before we merge? It's fine if there are, I don't mean to pressure but happy to help get this across the line any way I can.

@jpogran
Copy link
Contributor Author

jpogran commented Apr 5, 2021

I don't think there are any other open items. We have successfull runs in puppet-agent, bolt and pdk, I think this is ready for merge. 🚀 If everyone is ok with it, I'll self merge this

The patch Josh is mentioning would be a separate PR.

@lucywyman
Copy link
Contributor

Holding off on merging this until Puppet 7.6.0 is released later this month.

@joshcooper
Copy link
Collaborator

We had to bump to 2.7.3 for a ruby cve and I think commit 6a6886d can be dropped due to the issue being fixed upstream.

jpogran and others added 3 commits April 26, 2021 09:51
@jpogran
(PA-3526) Patch Ruby 2.7.2 to avoid realpath in require
552f10e 
Tracked in https://tickets.puppetlabs.com/browse/PA-3526
@jpogran
(maint) update vanagon
609a354
@lucywyman @jpogran
Add new Windows 2019 platform to dtrace ignore arrays
a2ccf66 
This updates the list of platforms that don't have dtrace to include the
new Windows 2019 platform, so that puppet-runtime can be built for
Windows 2019.
@jpogran jpogran force-pushed the ruby-2.7.2-perf-patches branch from d815fb3 to a2ccf66 Compare April 26, 2021 13:52
@jpogran
Copy link
Contributor Author

jpogran commented Apr 26, 2021

Commit 6a6886d removed, rebased and synced with master

@lucywyman
Copy link
Contributor

Is this ready to remove 'do not merge' label?

@joshcooper
Copy link
Collaborator

joshcooper commented Apr 27, 2021 •
edited
Loading

Is the "Add new Windows 2019 platform to dtrace ignore arrays" commit needed or is it for a different problem? Ah I see the comment in #414 (comment)

@joshcooper joshcooper changed the title Ruby 2.7.2 performance patches (PA-3526) Ruby 2.7.2 performance patches Apr 27, 2021
@joshcooper joshcooper merged commit c56420a into puppetlabs:master Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshcooper joshcooper joshcooper approved these changes

+2 more reviewers

@mihaibuzgau mihaibuzgau mihaibuzgau approved these changes

@lucywyman lucywyman lucywyman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jpogran @mihaibuzgau @lucywyman @joshcooper